Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate proto generation to buf #1139

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Migrate proto generation to buf #1139

wants to merge 2 commits into from

Conversation

afritzler
Copy link
Member

@afritzler afritzler commented Oct 25, 2024

Proposed Changes

  • Migrate proto generation to buf
  • Update linter configuration
  • Fix linting issues

You can now run the proto generation via:

make proto

This will run the following command on the project:

buf generate

The buf configuration can be found in the bug.gen.yaml and buf.yaml files in the project root.

Fixes #1138

@afritzler afritzler requested a review from a team as a code owner October 25, 2024 11:56
@github-actions github-actions bot added size/XXL enhancement New feature or request labels Oct 25, 2024
@afritzler afritzler force-pushed the enh/proto-generation branch 3 times, most recently from 109222f to 5ade639 Compare October 28, 2024 13:45
- Migrate proto generation to `buf`
- Update linter configuration
- Fix linting issues

linters:
disable-all: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we first disable all linters? Isn't it possible to invert it? Enable all and disable the not needed linters?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @lukasfrank, other projects (looked at prometheus and some grafana projects) use the same pattern to only disable unwanted linters. Golang CI Lint also adds linters with new updates which would be removed, as such you have the overhead to also review those changes as they come in. With an inverse you will eventually see failed golangcilint runs and can decide if this a valid issue or if the linter (or its check) should be disabled.

As per golangcilint documentation both enable-all and disable-all fields are false by default to achieve both patterns :)

version: v2
plugins:
# The protoc-gen-go stubs are required for grpc-go
- remote: buf.build/protocolbuffers/go
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we are calling an external API by using a remote plugin.
We might want to get rid of it.

➜  ironcore git:(enh/proto-generation) ✗ buf generate
Failure: too many requests

Please see https://buf.build/docs/bsr/rate-limits for details about BSR rate limiting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size/XXL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate from github.com/gogo/protobuf/protoc-gen-gogo in make proto
3 participants